fix[next-dace]: Added a Check for Symbol Conflicts Upon relocation#2472
Conversation
This is essentially the content of the file from `symbol_aliasing_issue_in_dataflow_if_mover__2026_02_03` (4315d0c), see also GridTools#2472. Furthermore, this PR also enables the transformation again in the auto optimizer.
|
cscs-ci run |
src/gt4py/next/program_processors/runners/dace/transformations/move_dataflow_into_if_body.py
Outdated
Show resolved
Hide resolved
src/gt4py/next/program_processors/runners/dace/transformations/move_dataflow_into_if_body.py
Outdated
Show resolved
Hide resolved
| # TODO(phimuell): Because of C++ scoping rules it might be possible | ||
| # to skip this step, i.e. not add them to the set. | ||
| assert node_to_check is not enclosing_map | ||
| requiered_symbols |= set(node_to_check.map.params) |
There was a problem hiding this comment.
what about the symbols used in the map range? could they possibly also clash?
There was a problem hiding this comment.
This is a bit confusing they (Map parameter symbols) are in fact included.
free_symbols only returns (AFAIK) only the symbols that need to be defined, thus considering a Map as a whole, they are not free, because the Map itself provides/supplies them.
If we would skip this, i.e. not care about them, then we would allow that outside the Map a symbol exists, that inside of the Map would have a different meaning.
Now that I write these lines it is probably okay to skip this, because of the scoping rules.
What do you think.
There was a problem hiding this comment.
You are right that the map parameters could redefine symbols that have a different meaning outside the map scope. So it should be OK to skip map.params.
However, my original comment was about map.range.free_symbols. There, it should not be allowed to have name clashes.
There was a problem hiding this comment.
map.range.free_symbols are already included in the free_symbols we perform on the subgraph object, because they are free symbols of the subgraph, while the map paramater are not, they are supplied by the Map itself and do not need to be supplied by the outside.
There was a problem hiding this comment.
I see, thanks for the explanation.
src/gt4py/next/program_processors/runners/dace/transformations/move_dataflow_into_if_body.py
Outdated
Show resolved
Hide resolved
edopao
changed the title
edopao
changed the title
…_in_dataflow_if_mover__2026_02_03
|
Results for Bluline: 
There is some variablility, this is why I run it twice, but I think it is okay. |
|
Also checked the graupel performance (on |
2 participants
When dataflow is relocated it might happen that some symbols clashes, i.e. there is a symbol, e.g.
n, defined in both the parent and theif_block, but in both time with slightly different meanings. This PR adds the functionality to overcome this clash, but adds a check to spot it.